Add DD_DASHBOARD_FORCE_SYNC_PERIOD environment variable #2184

puretension · 2025-09-18T15:13:18Z

What does this PR do?

Adds DD_DASHBOARD_FORCE_SYNC_PERIOD environment variable to configure DatadogDashboard controller sync frequency, following the same pattern as DD_MONITOR_FORCE_SYNC_PERIOD.

Motivation

Fixes #2179 - DatadogDashboard controller has a hardcoded 60-minute force sync period, unlike DatadogMonitor which supports DD_MONITOR_FORCE_SYNC_PERIOD. This means dashboard drift from manual UI changes can persist for up to 60 minutes.

Additional Notes

Implementation follows the exact same pattern as DatadogMonitor controller
Defaults to 60 minutes if environment variable is not set
Includes error handling for invalid values
All existing tests pass

Minimum Agent Versions

No minimum version requirements - this is an operator-only change.

Agent: N/A
Cluster Agent: N/A

Describe your test plan

Verified existing DatadogDashboard tests still pass
Tested environment variable parsing logic with valid/invalid values
Confirmed lint and build checks pass

Checklist

PR has at least one valid label: enhancement
PR has a milestone or the qa/skip-qa label

…#2179 Signed-off-by: puretension <[email protected]>

puretension · 2025-09-18T15:22:13Z

Hi! Could someone please add the 'enhancement' label and either a milestone or the 'qa/skip-qa' label to this PR?
The CI is failing due to missing labels/milestone.

Signed-off-by: puretension <[email protected]>

orkhanM

Should any documentation be updated?

orkhanM · 2025-09-19T10:51:57Z

internal/controller/datadogdashboard/controller.go

 			logger.Info("DatadogDashboard manifest has changed")
 			shouldUpdate = true
-		} else if instance.Status.LastForceSyncTime == nil || ((defaultForceSyncPeriod - now.Sub(instance.Status.LastForceSyncTime.Time)) <= 0) {
+		} else if instance.Status.LastForceSyncTime == nil || ((forceSyncPeriod - now.Sub(instance.Status.LastForceSyncTime.Time)) <= 0) {


@puretension Thanks for putting this together! It looks good I think but it seems like this doesn't follow the same pattern as the monitors do exactly. I haven't dived too deeply into the patterns used in other crd controllers here but the Monitor controller seems to check the Monitor specific MonitorLastForceSyncTime rather than the generic LastForceSyncTime:

datadog-operator/internal/controller/datadogmonitor/controller.go

Lines 154 to 178 in 900987e

} else if instance.Status.MonitorLastForceSyncTime == nil || (forceSyncPeriod-now.Sub(instance.Status.MonitorLastForceSyncTime.Time)) <= 0 {

// Periodically force a sync with the API monitor to ensure parity

// Get monitor to make sure it exists before trying any updates. If it doesn't, set shouldCreate

m, err = r.get(instance, newStatus)

if err != nil {

logger.Error(err, "error getting monitor", "Monitor ID", instance.Status.ID)

if strings.Contains(err.Error(), ctrutils.NotFoundString) {

shouldCreate = true

}

} else {

shouldUpdate = true

}

} else if instance.Status.MonitorStateLastUpdateTime == nil || (defaultRequeuePeriod-now.Sub(instance.Status.MonitorStateLastUpdateTime.Time)) <= 0 {

// If other conditions aren't met, and we have passed the defaultRequeuePeriod, then update monitor state

// Get monitor to make sure it exists before trying any updates. If it doesn't, set shouldCreate

m, err = r.get(instance, newStatus)

if err != nil {

logger.Error(err, "error getting monitor", "Monitor ID", instance.Status.ID)

if strings.Contains(err.Error(), ctrutils.NotFoundString) {

shouldCreate = true

}

}

updateMonitorState(m, now, newStatus)

}

}

The MonitorLastForceSyncTime seems to be set whenever a monitor is synced. I'm not sure if having a Monitor specific variable there was intentional or not or if the maintainers care about consistency in this case but thought it'd be worthwhile to bring to your attention.

@orkhanM Great catch! You're absolutely right about the consistency issue.

Looking at the Monitor controller pattern you referenced,
I can see it uses MonitorLastForceSyncTime for the force sync check.
I'll update the Dashboard implementation to use DashboardLastForceSyncTime
to match this pattern for consistency across controllers.

Thanks for pointing this out!

puretension · 2025-09-19T12:37:27Z

Should any documentation be updated?

I don't think any documentation updates are needed for this change
since it's an internal API field rename for consistency with the Monitor controller pattern.
But I'm not entirely sure😅 would appreciate the maintainers' input on this!

dd-octo-sts · 2025-10-09T10:07:34Z

This pull request has been automatically marked as stale because it has not had activity in the past 15 days.

It will be closed in 30 days if no further activity occurs. If this pull request is still relevant, adding a comment or pushing new commits will keep it open. Also, you can always reopen the pull request if you missed the window.

Thank you for your contributions!

tbavelier

Hello @puretension ,

Thank you for the contribution! While I understand the CRD was changed to be consistent with the monitor one, such changes could break customers that do not update the CRD while updating the operator by trying to access a field that does not exist. Moreover, changing a CRD (_types.go) should cause more files to be changed (make generate should show these changes).

Could you please remove the last commit ? I think docs/datadog_dashboard.md could also benefit from this change being documented.

Thanks!

Signed-off-by: puretension <[email protected]>

puretension · 2025-10-17T12:52:26Z

Hi @tbavelier,

Thank you for the detailed feedback and for explaining the compatibility concerns with CRD changes. I've removed the last commit that modified the CRD to avoid potential issues for customers who might update the operator without updating the CRD. I also added documentation for the DD_DASHBOARD_FORCE_SYNC_PERIOD environment variable in docs/datadog_dashboard.md as you suggested.

I appreciate you taking the time to explain why the CRD change could be problematic and the importance of running make generate when modifying types. Please let me know if there's anything else I missed or if I should make any other adjustments to the implementation.

Thanks again for your guidance!

tbavelier

Code is looking fine, could you please update the documentation based on my comment ? Thanks in advance

docs/datadog_dashboard.md

…_FORCE_SYNC_PERIOD in step 3 Signed-off-by: puretension <[email protected]>

Add DD_DASHBOARD_FORCE_SYNC_PERIOD environment variable Fixes DataDog…

760353f

…#2179 Signed-off-by: puretension <[email protected]>

puretension requested a review from a team as a code owner September 18, 2025 15:13

levan-m added the enhancement New feature or request label Sep 18, 2025

levan-m added this to the v1.20.0 milestone Sep 18, 2025

trigger CI checks after labeling and milestone

a43835b

Signed-off-by: puretension <[email protected]>

orkhanM reviewed Sep 19, 2025

View reviewed changes

dd-octo-sts bot added the stale label Oct 9, 2025

tbavelier requested changes Oct 17, 2025

View reviewed changes

tbavelier modified the milestones: v1.20.0, v1.21.0 Oct 17, 2025

Add DD_DASHBOARD_FORCE_SYNC_PERIOD documentation

d7ce92c

puretension force-pushed the add-dashboard-force-sync-period-env branch from d91dec2 to d7ce92c Compare October 17, 2025 12:50

puretension requested a review from a team as a code owner October 17, 2025 12:50

Add DD_DASHBOARD_FORCE_SYNC_PERIOD documentation

7781b7c

Signed-off-by: puretension <[email protected]>

Merge branch 'main' into add-dashboard-force-sync-period-env

5b38e10

tbavelier requested changes Oct 20, 2025

View reviewed changes

docs/datadog_dashboard.md Show resolved Hide resolved

docs: fix duplicated configuration section and reference DD_DASHBOARD…

1ab85a8

…_FORCE_SYNC_PERIOD in step 3 Signed-off-by: puretension <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add DD_DASHBOARD_FORCE_SYNC_PERIOD environment variable #2184

Add DD_DASHBOARD_FORCE_SYNC_PERIOD environment variable #2184

Uh oh!

puretension commented Sep 18, 2025

Uh oh!

puretension commented Sep 18, 2025

Uh oh!

orkhanM left a comment

Uh oh!

orkhanM Sep 19, 2025

Uh oh!

puretension Sep 19, 2025

Uh oh!

puretension commented Sep 19, 2025

Uh oh!

dd-octo-sts bot commented Oct 9, 2025

Uh oh!

tbavelier left a comment

Uh oh!

puretension commented Oct 17, 2025

Uh oh!

tbavelier left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	} else if instance.Status.MonitorLastForceSyncTime == nil \|\| (forceSyncPeriod-now.Sub(instance.Status.MonitorLastForceSyncTime.Time)) <= 0 {
	// Periodically force a sync with the API monitor to ensure parity
	// Get monitor to make sure it exists before trying any updates. If it doesn't, set shouldCreate
	m, err = r.get(instance, newStatus)
	if err != nil {
	logger.Error(err, "error getting monitor", "Monitor ID", instance.Status.ID)
	if strings.Contains(err.Error(), ctrutils.NotFoundString) {
	shouldCreate = true
	}
	} else {
	shouldUpdate = true
	}
	} else if instance.Status.MonitorStateLastUpdateTime == nil \|\| (defaultRequeuePeriod-now.Sub(instance.Status.MonitorStateLastUpdateTime.Time)) <= 0 {
	// If other conditions aren't met, and we have passed the defaultRequeuePeriod, then update monitor state
	// Get monitor to make sure it exists before trying any updates. If it doesn't, set shouldCreate
	m, err = r.get(instance, newStatus)
	if err != nil {
	logger.Error(err, "error getting monitor", "Monitor ID", instance.Status.ID)
	if strings.Contains(err.Error(), ctrutils.NotFoundString) {
	shouldCreate = true
	}
	}
	updateMonitorState(m, now, newStatus)
	}
	}

Add DD_DASHBOARD_FORCE_SYNC_PERIOD environment variable #2184

Are you sure you want to change the base?

Add DD_DASHBOARD_FORCE_SYNC_PERIOD environment variable #2184

Uh oh!

Conversation

puretension commented Sep 18, 2025

What does this PR do?

Motivation

Additional Notes

Minimum Agent Versions

Describe your test plan

Checklist

Uh oh!

puretension commented Sep 18, 2025

Uh oh!

orkhanM left a comment

Choose a reason for hiding this comment

Uh oh!

orkhanM Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

puretension Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

puretension commented Sep 19, 2025

Uh oh!

dd-octo-sts bot commented Oct 9, 2025

Uh oh!

tbavelier left a comment

Choose a reason for hiding this comment

Uh oh!

puretension commented Oct 17, 2025

Uh oh!

tbavelier left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants